packages/ai-byok/tests/react.test.tsx (1)

6-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Merge duplicate ../src/index type imports.

Static analysis flags import/no-duplicates for the two separate type-only imports from ../src/index.

🧹 Proposed fix

-import type { Keyring } from '../src/index'
-import type { KeyringStorage } from '../src/index'
+import type { Keyring, KeyringStorage } from '../src/index'

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ai-byok/tests/react.test.tsx` around lines 6 - 7, The React test
file has duplicate type-only imports from the same module, triggering
import/no-duplicates. Update the import section in react.test.tsx to merge the
Keyring and KeyringStorage type imports into a single import statement from
../src/index, keeping the existing type-only form and preserving the referenced
symbols.

Source: Linters/SAST tools

packages/ai-byok/tests/byok.test.ts (1)

1-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Fix import order/sort lint errors.

Static analysis flags: byokHeaderName out of alphabetical order in the named import on Line 2, and the type import on Line 3 should be ordered after the ../src/client/passkey import.

🧹 Proposed fix for import ordering

-import { byokFetch, byokHeaders, byokHeaderName, withByok } from '../src/index'
-import type { Keyring } from '../src/index'
-import {
-  byokMissing,
-  getByokKey,
-  isByokMissingBody,
-  maskKey,
-  scrubSecrets,
-} from '../src/server'
-import { memoryStorage } from '../src/client/storage'
-import {
-  decryptKeyring,
-  deriveAesKey,
-  encryptKeyring,
-} from '../src/client/passkey'
+import { byokFetch, byokHeaderName, byokHeaders, withByok } from '../src/index'
+import {
+  byokMissing,
+  getByokKey,
+  isByokMissingBody,
+  maskKey,
+  scrubSecrets,
+} from '../src/server'
+import { memoryStorage } from '../src/client/storage'
+import {
+  decryptKeyring,
+  deriveAesKey,
+  encryptKeyring,
+} from '../src/client/passkey'
+import type { Keyring } from '../src/index'

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ai-byok/tests/byok.test.ts` around lines 1 - 16, The test file
import ordering is violating lint rules: the named import from byok helpers has
an out-of-order symbol, and the type-only import is placed before the
client/passkey imports. Reorder the imports in byok.test.ts so the named
specifiers in the ../src/index import are alphabetized and the type import for
Keyring is moved to the correct position after the ../src/client/passkey import,
keeping the existing symbols and groupings intact.

Source: Linters/SAST tools

packages/ai-byok/src/client/passkey.ts (1)

299-322: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Guard ensureKey against concurrent ceremonies.

The doc comment promises "exactly one WebAuthn ceremony," but because the cachedKey/cachedMeta assignment is await-gated, two overlapping calls (e.g. a save racing a load, or rapid double-save) both observe an empty cache, both call idbGet, and both run a full ceremony. In the register branch this can mint two passkeys with only the last record persisted, orphaning the other. Memoize the in-flight promise so concurrent callers share one ceremony.

♻️ Sketch

   let cachedKey: CryptoKey | null = null
   let cachedMeta: {
     credentialId: ArrayBuffer
     salt: Uint8Array<ArrayBuffer>
   } | null = null
+  let pending: Promise<{
+    key: CryptoKey
+    credentialId: ArrayBuffer
+    salt: Uint8Array<ArrayBuffer>
+  }> | null = null
 
   async function ensureKey(): Promise<{
     key: CryptoKey
     credentialId: ArrayBuffer
     salt: Uint8Array<ArrayBuffer>
   }> {
     if (cachedKey && cachedMeta) {
       return { key: cachedKey, ...cachedMeta }
     }
-    const existing = await idbGet(dbName)
-    ...
-    return { key: cachedKey, ...cachedMeta }
+    if (!pending) {
+      pending = (async () => {
+        // ...existing register/unlock logic, sets cachedKey/cachedMeta...
+        return { key: cachedKey!, ...cachedMeta! }
+      })().finally(() => { pending = null })
+    }
+    return pending
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ai-byok/src/client/passkey.ts` around lines 299 - 322, The ensureKey
helper currently allows overlapping calls to start separate WebAuthn ceremonies
because cachedKey/cachedMeta are only populated after awaited work completes.
Update ensureKey to memoize and share a single in-flight promise so concurrent
callers (for example load and save, or rapid double-save) all await the same
ceremony; reuse the existing cachedKey/cachedMeta fast path, but ensure the
first unresolved call to ensureKey owns the idbGet/registerPasskey flow and
later calls return that same promise until it settles.

packages/ai-byok/src/client/validate.ts (1)

22-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

No timeout on the validation fetch.

fetch has no AbortSignal/timeout, so a stalled network or slow-to-respond provider endpoint leaves the caller's await (and, per byok-context.tsx, the UI's validating state) hanging indefinitely with no way for the user to recover short of a page reload.

⏱️ Proposed fix

 export async function validateKey(
   provider: ProviderId,
   key: string,
 ): Promise<ValidationStatus> {
   const config = providerValidateConfig(provider)
   if (!config) return 'unsupported'

-  const response = await fetch(config.url, {
-    method: 'GET',
-    headers: config.headers(key),
-  })
+  const response = await fetch(config.url, {
+    method: 'GET',
+    headers: config.headers(key),
+    signal: AbortSignal.timeout(10_000),
+  })

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ai-byok/src/client/validate.ts` around lines 22 - 40, The
validateKey() fetch call currently has no timeout, so a slow or stalled provider
can leave the request and validating state hanging forever. Update validateKey()
in validate.ts to use an AbortSignal with a timeout (for example via
AbortController) around the fetch call, and ensure timeout aborts are handled as
a validation failure instead of hanging. Keep the existing
providerValidateConfig(), response handling, and ValidationStatus behavior
intact.

packages/ai-byok/src/server/byok-missing.ts (1)

39-44: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Type guard doesn't fully validate its asserted shape.

isByokMissingBody only checks error.type, not error.provider/error.message, yet it asserts the full ByokMissingBody interface (Line 4-10). Downstream, with-byok.ts calls onMissingKey(body.error.provider) (Line 23) trusting this is a valid ProviderId — an untrusted/malformed 401 body could pass this guard and hand a bad value to the caller's callback.

♻️ Proposed fix to validate provider/message

 import type { ProviderId } from '../shared/providers'
+import { isProviderId } from '../shared/providers'

 ...

 export function isByokMissingBody(value: unknown): value is ByokMissingBody {
   if (typeof value !== 'object' || value === null) return false
   const { error } = value as { error?: unknown }
   if (typeof error !== 'object' || error === null) return false
-  return (error as { type?: unknown }).type === 'byok_missing'
+  const { type, provider, message } = error as {
+    type?: unknown
+    provider?: unknown
+    message?: unknown
+  }
+  return (
+    type === 'byok_missing' &&
+    typeof provider === 'string' &&
+    isProviderId(provider) &&
+    typeof message === 'string'
+  )
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ai-byok/src/server/byok-missing.ts` around lines 39 - 44,
`isByokMissingBody` currently only verifies `error.type`, so it can accept
malformed bodies while claiming to be `ByokMissingBody`. Update the guard in
`byok-missing.ts` to fully validate the asserted shape by checking
`error.provider` and `error.message` alongside `error.type`, ensuring `provider`
matches a valid `ProviderId`-like string and `message` is a string before
returning true. Keep the checks inside `isByokMissingBody` so `with-byok.ts` can
safely call `onMissingKey(body.error.provider)` without trusting unvalidated
data.

packages/ai-byok/src/react/byok-key-manager.tsx (2)

101-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

No confirmation before clearing a saved key.

A single click on "Clear" permanently removes the stored key with no undo/confirm step. Worth a lightweight confirm (or a two-step "Clear?" toggle) to prevent accidental deletion, though the user can always re-enter the key.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ai-byok/src/react/byok-key-manager.tsx` around lines 101 - 107, The
Clear action in the byok-key-manager component triggers immediate key deletion
with no safeguard, so add a lightweight confirmation step before calling
clearKey(provider). Update the button behavior in ByokKeyManager to require a
second explicit confirmation (or a confirm dialog/toggle) before clearing the
saved key, while keeping the existing clearKey provider flow unchanged once
confirmed.

---

88-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant 'masked' in status check.

hasKey already excludes only the 'empty' state, and every non-'empty' KeyStatus variant carries masked (per byok-context.tsx's KeyStatus union). The extra narrowing is harmless but adds noise.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ai-byok/src/react/byok-key-manager.tsx` at line 88, Remove the
redundant masked property check in byok-key-manager.tsx’s render condition: the
hasKey guard already covers all non-empty KeyStatus values, so update the
conditional around the masked UI branch to rely on hasKey alone and keep the
logic aligned with the KeyStatus union defined in byok-context.tsx.

packages/ai-byok/src/react.ts (1)

4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Exporting the raw ByokContext bypasses the useByok safety guard.

Consumers can call useContext(ByokContext) directly and get undefined silently instead of the descriptive error thrown by useByok. Consider keeping ByokContext internal (only exported for testing) and steering public consumers exclusively to useByok.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ai-byok/src/react.ts` at line 4, The public re-export in react.ts is
exposing ByokContext directly, which lets consumers bypass the safety checks in
useByok and receive undefined silently. Remove ByokContext from the public
export surface in react.ts, keep it internal or test-only in react/byok-context,
and ensure consumers are directed to use the useByok hook (and ByokProvider) for
all supported access paths.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes